Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Oct 22, 2024

Now the firefox and chrome both use BigInt for indexing 64-bit memories and tables this workaround is no longer needed.

@sbc100 sbc100 requested a review from dschuff October 22, 2024 14:57
@sbc100 sbc100 force-pushed the remove_toIndexType branch from 30271f3 to 0dd9f78 Compare October 22, 2024 17:14
@sbc100 sbc100 requested a review from kripken October 22, 2024 17:14
function toIndexType(x) {
if (MEMORY64 != 1) return x;
return `toIndexType(${x})`;
return to64(x);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we replace calls from toIndexType to to64? Or do we think the distinction is important/might matter later?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lean towards keeping the distinction, but just a thought.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I thought of doing that. Can always be done as followup I guess?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup sounds fine, but also I am not sure if that is worthwhile. Unless we are certain we won't care about the difference later (which I am not sure of myself).

@sbc100 sbc100 force-pushed the remove_toIndexType branch from 0dd9f78 to d239a63 Compare October 31, 2024 14:04
@sbc100 sbc100 enabled auto-merge (squash) October 31, 2024 14:04
@sbc100 sbc100 force-pushed the remove_toIndexType branch 5 times, most recently from 19669ae to 148a408 Compare November 1, 2024 23:04
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 1, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
@sbc100 sbc100 mentioned this pull request Nov 1, 2024
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from emscripten-core#22773
@sbc100 sbc100 force-pushed the remove_toIndexType branch from 148a408 to 13bcbe9 Compare November 4, 2024 20:25
sbc100 added a commit that referenced this pull request Nov 4, 2024
This involved using toIndexType in a few more places due to recent
wasm64 changes requiring BitInt in more places.

Split out from #22773
@sbc100 sbc100 force-pushed the remove_toIndexType branch 2 times, most recently from 1c7ab1b to 7b0634d Compare November 4, 2024 23:20
Now the firefox and chrome both use BigInt for indexing 64-bit memories
and tables this workaround is no longer needed.
@sbc100 sbc100 force-pushed the remove_toIndexType branch from 7b0634d to 8685f1a Compare November 5, 2024 00:48
@sbc100 sbc100 merged commit c560421 into emscripten-core:main Nov 5, 2024
28 checks passed
@sbc100 sbc100 deleted the remove_toIndexType branch November 5, 2024 04:50
sbc100 added a commit to sbc100/emscripten that referenced this pull request Nov 5, 2024
sbc100 added a commit that referenced this pull request Nov 5, 2024
Its a good job we kept `toIndexType` around in #22773 because it turns
out it does need different semantics to `to64` after all.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants